Module talk:Lua class
Globals
[edit]@Alexiscoutinho: Further to our discussion at Module talk:TableTools#Shouldn't isArray be more generic?, I had a quick look at this very nice module. There may be a couple of issues for you to contemplate. First, many modules require Module:No globals. If one of those modules tried to use something from here, it would be likely to crash because there are several globals in this module. Are they really needed, or should it return a table containing tables of functions + classes + instances? Second, are you aware that in function try_parser, calls to type()
are calling Lua's function, not the _G.type
modified function? Johnuniq (talk) 10:43, 11 July 2021 (UTC)
- @Johnuniq: The globals probably aren't required, but are extremely recommended. I would prefer if such modules dropped their use of Module:No globals if they were to use classes. This module is meant to fundamentally change how other modules are structured and coded so it's only fitting for them to switch to globals. I wasn't aware of that
type
bug. I knew I shouldn't call it below that function override, but I completely forgot that the parser called it during the pre-submit check. I haven't extensively tested this module by the way. I was hoping to initially test it through examples and actual usage in other basic type modules (upcoming). Alexiscoutinho (talk) 05:24, 12 July 2021 (UTC)- The problem is that people have no way of detecting typos which generate unintended globals, so Module:No_globals is fairly common and may be becoming more so. I don't use it because I have a script which shows globals. I don't mind, I just thought I would point out the issue. Re "type", I would rename the variable to
lua_type
so there cannot be any confusion in the code or in the reader. Johnuniq (talk) 05:31, 12 July 2021 (UTC)- @Johnuniq: Well, I think the ideal solution would be to open more exceptions (besides "arg") in Module:No globals and/or require it after Lua class. Alexiscoutinho (talk) 06:06, 12 July 2021 (UTC)
- That's up to you, I just wanted to make sure you were aware of the issue. At any rate, if you ever wanted, you could alter the way it works in the future. Johnuniq (talk) 07:26, 12 July 2021 (UTC)
- The convention with Lua is to minimize the use of globals. See [1] and also [2] which is the style guide used by LuaRocks. Gonnym (talk) 14:06, 13 July 2021 (UTC)
- That's up to you, I just wanted to make sure you were aware of the issue. At any rate, if you ever wanted, you could alter the way it works in the future. Johnuniq (talk) 07:26, 12 July 2021 (UTC)
- @Johnuniq: Well, I think the ideal solution would be to open more exceptions (besides "arg") in Module:No globals and/or require it after Lua class. Alexiscoutinho (talk) 06:06, 12 July 2021 (UTC)
- The problem is that people have no way of detecting typos which generate unintended globals, so Module:No_globals is fairly common and may be becoming more so. I don't use it because I have a script which shows globals. I don't mind, I just thought I would point out the issue. Re "type", I would rename the variable to
Sure, I'll try to minimize globals. Current new globals are:
class
issubclass
isinstance
try
Planned optional globals basic types are:
Alexiscoutinho (talk) 23:13, 21 July 2021 (UTC)
- If you want this module to be widely used, there should be no globals. Why are they needed? It's cleaner to use the "namespace" of a required module and write x.class (where x is the table returned by require('Module:Lua class')). Johnuniq (talk) 02:42, 22 July 2021 (UTC)
- Globals are a bad thing, please avoid them. As a bare minimum make it possible to override them. To clarify, globals make hard to test code and code that somehow use those globals become non-portable. 2001:4644:13BE:0:E955:D3DE:360E:9F1D (talk) 16:52, 18 December 2022 (UTC)
elseif
[edit]Several of the elseif
can be avoided, as the previous clause exit's from further evaluation. 2001:4644:13BE:0:E955:D3DE:360E:9F1D (talk) 16:56, 18 December 2022 (UTC)
assert
[edit]There are several occurrences of a test clause followed by an error statement. These can be replaced by assert
, thus avoiding the explicit if clause. That is assert(<test>, <message>)
. The only place where an explicit error might be correct is when a redirect on level is appropriate, that is the second argument to error(<message>, <level>)
. 2001:4644:13BE:0:E955:D3DE:360E:9F1D (talk) 17:01, 18 December 2022 (UTC)
libraryUtil.checkTypeMultiForIndex
[edit]There are several libraryUtil.checkTypeMultiForIndex
after argument is first used. 2001:4644:13BE:0:E955:D3DE:360E:9F1D (talk) 17:06, 18 December 2022 (UTC)
type
[edit]Reimplementing standard functions are a bad thing, it makes the code fragile, non-portable, and outright dangerous to use as it can make unrelated code running in the same context to fail. This has implications for Module:Lua_set in addition for the one on the subject page. 2001:4644:13BE:0:E955:D3DE:360E:9F1D (talk) 17:12, 18 December 2022 (UTC)
refactor class, possibly also constructor
[edit]The class
function is way to complex. The constructor
also border on being to complex. 2001:4644:13BE:0:E955:D3DE:360E:9F1D (talk) 18:29, 18 December 2022 (UTC)